Skip to content

Security hardening for observability packages#198

Merged
nikhilNava merged 11 commits intomainfrom
copilot/security-review-observability-packages
Mar 17, 2026
Merged

Security hardening for observability packages#198
nikhilNava merged 11 commits intomainfrom
copilot/security-review-observability-packages

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 16, 2026

  • Explore codebase and identify all unbounded collections in observability extensions
  • Implement bounded collections for LangChain tracer (_spans_by_run OrderedDict with _MAX_TRACKED_RUNS=10000)
  • Add bounded _output_messages list in OutputScope (cap at _MAX_OUTPUT_MESSAGES=5000)
  • Add unit tests for LangChain bounded collections
  • Add unit tests for OutputScope bounded messages
  • Run lint, format, and test suite to validate changes (178 passed)
  • Code review and security scan (0 alerts)

💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.

Copilot AI and others added 3 commits March 16, 2026 15:01
… utils

Replace all 30 assert statements in utils.py with equivalent
if-not-raise TypeError checks. This ensures type validation is not
silently stripped when Python runs with -O (optimized mode).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… asserts, and more

- Fix #1: Downgrade sensitive data logging from INFO to DEBUG in agent365_exporter.py
- Fix #2: Fix unpaired context.attach() in opentelemetry_scope.py add_baggage() by storing
  and detaching baggage tokens on scope end
- Fix #3: Add bounded OrderedDict caps to unbounded dicts in OpenAI trace_processor.py
- Fix #4: Replace 30 assert statements with proper TypeError raises in LangChain utils.py
- Fix #5: Log security warning when HTTP domain override is detected
- Fix #6: Warn when bearer token sent over non-HTTPS connection
- Fix #10: Respect Retry-After header and use exponential backoff in retries
- Fix #13: Rename reset() to _reset() in ObservabilityHostingManager
- Fix #15: Replace print() with logger.warning() in LangChain tracer_instrumentor.py

Co-authored-by: nikhilNava <211831449+nikhilNava@users.noreply.github.com>
@nikhilNava nikhilNava marked this pull request as ready for review March 16, 2026 16:31
@nikhilNava nikhilNava requested a review from a team as a code owner March 16, 2026 16:31
Copilot AI review requested due to automatic review settings March 16, 2026 16:31
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 16, 2026

⚠️ Deprecation Warning: The deny-licenses option is deprecated for possible removal in the next major release. For more information, see issue 997.

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Security hardening PR for observability packages addressing information disclosure, memory safety, reliability under -O, and credential protection across 6 packages.

Changes:

  • Downgraded sensitive data logging from INFO to DEBUG, added HTTP security warnings, exponential backoff with Retry-After support, and removed response body from error logs in the exporter
  • Replaced assert with if/raise TypeError in LangChain utils, converted print() to logger.warning(), and fixed context token leak in add_baggage() with proper detach on scope end
  • Converted unbounded dicts to capped OrderedDict in OpenAI trace processor and renamed reset() to _reset() for test-only API

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
agent365_exporter.py INFO→DEBUG logging, response body removal, exponential backoff, Retry-After support, bearer-over-HTTP warning
opentelemetry_scope.py Baggage token storage and reverse-order detach on scope end
utils.py (exporters) HTTP domain override security warning
trace_processor.py OrderedDict with cap for all tracking dicts, _cap_ordered_dict helper
utils.py (langchain) assert→if/raise TypeError for all 30 assertions
tracer_instrumentor.py print→logger.warning, logging import added
observability_hosting_manager.py reset()_reset()
utils.py (openai) Type hints widened to accept Mapping for OrderedDict compatibility
test_agent365_exporter.py Updated test expectations for DEBUG log level
test_observability_hosting_manager.py Updated to use _reset()

You can also share your feedback on Copilot code review. Take the survey.

Copilot AI and others added 2 commits March 16, 2026 16:39
Agent IDs and tenant IDs are not sensitive data and are useful for
debugging. Restore them in debug/error log messages. Also restore
truncated response text in HTTP error logs to help developers debug
failures.

Log levels remain at DEBUG (from the prior security fix).

Co-authored-by: nikhilNava <211831449+nikhilNava@users.noreply.github.com>
The method had an unpaired context.attach() that leaked context tokens.
Users should use BaggageBuilder.build() context manager instead, which
properly restores the previous context on exit.

Co-authored-by: nikhilNava <211831449+nikhilNava@users.noreply.github.com>
Co-authored-by: nikhilNava <211831449+nikhilNava@users.noreply.github.com>
@nikhilNava
Copy link
Copy Markdown
Contributor

@copilot fix the linting error
uv run --frozen ruff format --check .

…se TypeError with isinstance guards in langchain utils.py

Co-authored-by: nikhilNava <211831449+nikhilNava@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 16, 2026

@copilot fix the linting error
uv run --frozen ruff format --check .

Fixed in 596cc6a. The formatting issue in test_agent365_exporter.py is resolved. Also completed replacing all remaining raise TypeError instances in langchain utils.py with isinstance guards. Both ruff check and ruff format --check now pass cleanly.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • agent365.svc.cloud.microsoft
    • Triggering command: /home/REDACTED/work/Agent365-python/Agent365-python/.venv/bin/pytest pytest tests/observability/ -v --tb=short -m not integration -x -q (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

…ry_after function

Co-authored-by: nikhilNava <211831449+nikhilNava@users.noreply.github.com>
juliomenendez
juliomenendez previously approved these changes Mar 17, 2026
… review

Co-authored-by: nikhilNava <211831449+nikhilNava@users.noreply.github.com>
@fpfp100
Copy link
Copy Markdown

fpfp100 commented Mar 17, 2026

Comparing with nodejs security check, python is missing the following: Content recording gate (core scopes) defined but never checked, no bounded collections for LangChain, no per-attribute limit.

- Convert LangChain _spans_by_run from unbounded DictWithLock to
  bounded OrderedDict with _MAX_TRACKED_RUNS=10000 cap
- Add _cap_ordered_dict helper for FIFO eviction (matching OpenAI pattern)
- Add thread-safe lock usage for _spans_by_run in error handlers
- Add _MAX_OUTPUT_MESSAGES=5000 cap for OutputScope._output_messages
- Add unit tests for both bounded collections

Co-authored-by: nikhilNava <211831449+nikhilNava@users.noreply.github.com>
@nikhilNava nikhilNava enabled auto-merge (squash) March 17, 2026 19:22
@nikhilNava nikhilNava merged commit febcc09 into main Mar 17, 2026
9 checks passed
@nikhilNava nikhilNava deleted the copilot/security-review-observability-packages branch March 17, 2026 19:27
biswapm pushed a commit to biswapm/Agent365-python that referenced this pull request Mar 20, 2026
* Initial plan

* Replace assert statements with explicit TypeError raises in langchain utils

Replace all 30 assert statements in utils.py with equivalent
if-not-raise TypeError checks. This ensures type validation is not
silently stripped when Python runs with -O (optimized mode).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* security: fix sensitive data logging, context leak, unbounded memory, asserts, and more

- Fix microsoft#1: Downgrade sensitive data logging from INFO to DEBUG in agent365_exporter.py
- Fix microsoft#2: Fix unpaired context.attach() in opentelemetry_scope.py add_baggage() by storing
  and detaching baggage tokens on scope end
- Fix microsoft#3: Add bounded OrderedDict caps to unbounded dicts in OpenAI trace_processor.py
- Fix microsoft#4: Replace 30 assert statements with proper TypeError raises in LangChain utils.py
- Fix microsoft#5: Log security warning when HTTP domain override is detected
- Fix microsoft#6: Warn when bearer token sent over non-HTTPS connection
- Fix microsoft#10: Respect Retry-After header and use exponential backoff in retries
- Fix microsoft#13: Rename reset() to _reset() in ObservabilityHostingManager
- Fix microsoft#15: Replace print() with logger.warning() in LangChain tracer_instrumentor.py

Co-authored-by: nikhilNava <211831449+nikhilNava@users.noreply.github.com>

* Restore agent/tenant IDs and response text in exporter log messages

Agent IDs and tenant IDs are not sensitive data and are useful for
debugging. Restore them in debug/error log messages. Also restore
truncated response text in HTTP error logs to help developers debug
failures.

Log levels remain at DEBUG (from the prior security fix).

Co-authored-by: nikhilNava <211831449+nikhilNava@users.noreply.github.com>

* Remove add_baggage() from OpenTelemetryScope

The method had an unpaired context.attach() that leaked context tokens.
Users should use BaggageBuilder.build() context manager instead, which
properly restores the previous context on exit.

Co-authored-by: nikhilNava <211831449+nikhilNava@users.noreply.github.com>

* Security hardening for observability packages

Co-authored-by: nikhilNava <211831449+nikhilNava@users.noreply.github.com>

* Fix formatting in test_agent365_exporter.py and replace remaining raise TypeError with isinstance guards in langchain utils.py

Co-authored-by: nikhilNava <211831449+nikhilNava@users.noreply.github.com>

* Move _parse_retry_after to exporters/utils.py as standalone parse_retry_after function

Co-authored-by: nikhilNava <211831449+nikhilNava@users.noreply.github.com>

* Replace type(e).__name__ with str(e) in exporter error logging per PR review

Co-authored-by: nikhilNava <211831449+nikhilNava@users.noreply.github.com>

* feat: add bounded collections for LangChain tracer and OutputScope

- Convert LangChain _spans_by_run from unbounded DictWithLock to
  bounded OrderedDict with _MAX_TRACKED_RUNS=10000 cap
- Add _cap_ordered_dict helper for FIFO eviction (matching OpenAI pattern)
- Add thread-safe lock usage for _spans_by_run in error handlers
- Add _MAX_OUTPUT_MESSAGES=5000 cap for OutputScope._output_messages
- Add unit tests for both bounded collections

Co-authored-by: nikhilNava <211831449+nikhilNava@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: nikhilNava <211831449+nikhilNava@users.noreply.github.com>
Co-authored-by: Nikhil Navakiran <nikhil.navakiran@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants